-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix drift in scroll_overlay Android item alignment; small updates #13
Conversation
These changes are made by a plain `flutter pub get` with the current stable Flutter, 3.7.1.
This helps make it readable in an editor.
The heights on the Android side were systematically up to one physical pixel less than on the Flutter side, due to rounding. This made the items conspicuously out of alignment by the lower part of the first screen, even before any scrolling, making it impossible to tell whether a given scroll physics matched exactly or had a small discrepancy. Fix by using a tiny bit more math in the calculation, so that the rounding error doesn't accumulate.
@Piinks, could I interest you in reviewing this PR? After this I have some further enhancements to this test app queued up, including a handy graphical display of the two velocities which I've found quite helpful: I figure I'll send those as a second PR once this one is in (because this one is already doing several different small things, and combining everything together would make the resulting commit a bit messy to read.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me so happy thank you, the slight offset has always bothered me but I did not know why it was. Thank you!!
LGTM!
Great, thanks! After the weekend I'll turn those further changes into a second PR. |
OK, and done. I actually broke them up into three PRs:
I'm hoping that reduces, rather than increases, the work it takes to review them. |
Part of my work on flutter/flutter#119875.
The heights on the Android side were systematically up to one physical pixel less than on the Flutter side, due to rounding. This made the items conspicuously out of alignment by the lower part of the first screen, even before any scrolling, making it impossible to tell whether a given scroll physics matched exactly or had a small discrepancy.
Fix by using a tiny bit more math in the calculation, so that the rounding error doesn't accumulate.
Screenshots:
Also make a few small updates and other cleanups, in separate commits for ease of review. In particular, update pubspec.lock so that running the app with a current Flutter doesn't dirty the tree.
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.